Skip to content

MySQL Source Versioning V2 - reorganized boogaloo - #36333

Merged
patrickwwbutler merged 6 commits into
MaterializeInc:mainfrom
patrickwwbutler:patrick/mysql-fixes-reorganized
May 7, 2026
Merged

MySQL Source Versioning V2 - reorganized boogaloo#36333
patrickwwbutler merged 6 commits into
MaterializeInc:mainfrom
patrickwwbutler:patrick/mysql-fixes-reorganized

Conversation

@patrickwwbutler

@patrickwwbutler patrickwwbutler commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

This PR effectively re-implements source versioning for mysql after we reverted the initial implementation last week due to decoding problems with exclude columns resulting in an incident. In doing so, it fixes a number of issues that existed with the first implementation, including:
https://github.com/MaterializeInc/database-issues/issues/11312
https://github.com/MaterializeInc/database-issues/issues/11313
https://github.com/MaterializeInc/database-issues/issues/11315

And provides a safer mechanism for handling schema changes and changes to the binlog_row_metadata MySQL system variable.

The first commit is roughly the changes in #36253 which should be merged first, and is required for the other changes.

Second commit updates the decoding logic based on the binlog metadata setting at source creation

Third commit updates the logic to verify mysql schemas with the schemas in the upstream, allowing for certain types of schema changes when binlog_row_metadata is FULL.

Fourth commit contains docs for how to make schema changes to your mysql source without downtime in materialize.

@patrickwwbutler
patrickwwbutler requested a review from a team April 29, 2026 18:11
@patrickwwbutler
patrickwwbutler requested review from a team as code owners April 29, 2026 18:11
ublubu
ublubu reviewed Apr 29, 2026
View reviewed changes

@ublubu ublubu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a first pass at the "decode binlog rows by column name" commit. I'll look at the tests more carefully tomorrow.

Comment thread src/mysql-util/src/decoding.rs Outdated
Comment thread src/mysql-util/src/decoding.rs Outdated
Comment thread src/mysql-util/src/decoding.rs Outdated
Comment thread src/mysql-util/src/decoding.rs
Comment thread src/mysql-util/src/decoding.rs Outdated
Comment thread src/mysql-util/src/decoding.rs
kay-kim
kay-kim reviewed Apr 29, 2026
View reviewed changes
Comment thread doc/user/content/ingest-data/mysql/source-versioning.md Outdated
def-
def- reviewed Apr 30, 2026
View reviewed changes
Comment thread src/mysql-util/src/decoding.rs Outdated
Comment thread src/mysql-util/src/desc.rs Outdated
martykulma
martykulma reviewed Apr 30, 2026
View reviewed changes

@martykulma martykulma left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! There are some additional improvements to make; partly my fault - i didn't think through the strict vs. lenient checks in purification (sorry!).

Comment thread src/sql/src/pure.rs Outdated
Comment thread src/mysql-util/src/decoding.rs Outdated
@martykulma

martykulma commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

I just realized that test/mysql-cdc-resumption/mzcompose.py has verification disabled too - should one of these be re-enabled?

    # TODO: database-issues#7683: one of the two following commands must succeed
    # run_testdrive_files(c, "verify-rows-after-restore-t1.td")
    # run_testdrive_files(c, "verify-source-failed.td")

@patrickwwbutler
patrickwwbutler force-pushed the patrick/mysql-fixes-reorganized branch from 478081e to 090552c Compare April 30, 2026 20:24
patrickwwbutler and others added 4 commits May 5, 2026 09:49
@patrickwwbutler @claude
mysql: decode binlog rows by column name when binlog_full_metadata=true
b1d06b8 
Refactors `pack_mysql_row()` to accept `gtid_set` and `binlog_full_metadata`
parameters. When `binlog_full_metadata=true`, columns are matched by name from
the wire row to the table descriptor (safe under reordering). When false,
falls back to position-based matching (original behavior, required for
`binlog_row_metadata=MINIMAL`). Adds diagnostic helpers `decode_error()` and
`describe_row_shape()` for richer error context.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@patrickwwbutler @claude
mysql: verify schema compatibility using column names when binlog_ful…
139ba39 
…l_metadata=true

Adds a `full_metadata: bool` parameter to `MySqlTableDesc::determine_compatibility()`.
When true, columns are matched by name (allowing upstream reordering and safe
addition of new columns). When false, uses the original positional prefix check.
`verify_schemas()` now passes `output.binlog_full_metadata` to drive the choice.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@patrickwwbutler
check optional metadata in handle_rows_event instead of decoding
6080b0b
@patrickwwbutler
update desc schema compat logic to mirror decoding, update decoding c…
bbce9d4 
…omments
@patrickwwbutler
patrickwwbutler force-pushed the patrick/mysql-fixes-reorganized branch from 090552c to bbce9d4 Compare May 5, 2026 17:07
@patrickwwbutler

patrickwwbutler commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

I just realized that test/mysql-cdc-resumption/mzcompose.py has verification disabled too - should one of these be re-enabled?

    # TODO: database-issues#7683: one of the two following commands must succeed
    # run_testdrive_files(c, "verify-rows-after-restore-t1.td")
    # run_testdrive_files(c, "verify-source-failed.td")

Ah this is actually the test that is flaky because of the DROP-> re-CREATE race condition, which isn't fixed quite yet, so it should stay disabled for now

@patrickwwbutler
patrickwwbutler force-pushed the patrick/mysql-fixes-reorganized branch from c0a6864 to 10b4c82 Compare May 5, 2026 18:32
@patrickwwbutler
patrickwwbutler force-pushed the patrick/mysql-fixes-reorganized branch from 10b4c82 to 725768c Compare May 5, 2026 18:59
martykulma
martykulma reviewed May 6, 2026
View reviewed changes
Comment thread src/storage/src/source/mysql/replication/events.rs Outdated
Comment thread src/storage/src/source/mysql/snapshot.rs
Comment thread src/storage/src/source/mysql/replication/events.rs
Comment on lines +305 to +309
Err(DataflowError::from(DefiniteError::ValueDecodeError(
format!(
"Table {0} was created with binlog_row_metadata=FULL but binlog_row_metadata has since been set to a different value, meaning we cannot reliably decode the columns",
output.table_name
),

@martykulma martykulma May 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We never mark the export(s) as borked (ctx.errored_outputs), which means we will continue to emit errors as long as the setting is incorrect. Once the customer corrects it, there isn't a way to recover for MZ, but we will try!

Consider we have source will full metadata, a row with a value A that sees some updates and is deleted, and interleaved someone accidentally changes row metadata:

A -> B    .... A:-1 B:+1
------------------------------------ metadata: FULL -> MINIMAL
B -> C    .... Err(B):-1 Err(C):+1
------------------------------------ metadata: MINIMAL -> FULL
C -> NULL .... C:-1

@patrickwwbutler patrickwwbutler May 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like something that is fundamentally unrecoverable, no? You mean that by adding it to ctx.errored_outputs we and ensuring that it cannot be recovered, and also that the source is more visibly broken, I assume?

@martykulma martykulma May 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, by adding it - we stop processing events for it. The last thing appended would be the error. I believe we do this for the MySQL DDL errors, and you should also find it in PG.

martykulma
martykulma approved these changes May 7, 2026
View reviewed changes

@martykulma martykulma left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice - lgtm!

@patrickwwbutler

patrickwwbutler commented May 7, 2026

Copy link
Copy Markdown
Contributor Author

triggering a nightly before merging, fingers crossed

@patrickwwbutler
patrickwwbutler merged commit 47a81d6 into MaterializeInc:main May 7, 2026
174 checks passed
@peterdukelarsen peterdukelarsen mentioned this pull request Jun 22, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kay-kim kay-kim kay-kim left review comments
@def- def- def- left review comments
@martykulma martykulma martykulma approved these changes
@ohbadiah ohbadiah Awaiting requested review from ohbadiah ohbadiah was automatically assigned from MaterializeInc/adapter
@DAlperin DAlperin Awaiting requested review from DAlperin
@ublubu ublubu Awaiting requested review from ublubu

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@patrickwwbutler @martykulma @kay-kim @def- @ublubu